-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: add beforeMount callback to MountOptions
#17515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 642f967 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
It's great that you put something together so quickly!
This was the main reason I suggested a function alternative for |
This is just another name...you could've declared an effect also inside your |
|
However I just realized a problem...effects declared in here don't get triggered which could leak memory...investigating |
|
Fixed it |
|
Alternatively, we could solve this without adding any new API at all: #16997 (comment) |
Closes #16997
Just an experiment...the comment was proposing overloading
contextbut I think that would be bad for two reasons:This could also have some interesting side effects like being able to register an
effectthat is in context of the component but outside of it...dunno maybe could be useful for testing.Didn't add a test because we don't have test for
mount...but...should we?Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint